Skip to content

Conversation

@amnn
Copy link
Contributor

@amnn amnn commented Dec 3, 2025

Description

Clap requires clauses refer to other flags by their field name, not how they appear as a flag.

Test plan

The clickhouse-sui-indexer example failed to run because of this issue, and now it runs.


Release notes

Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.

For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.

  • Protocol:
  • Nodes (Validators and Full nodes):
  • gRPC:
  • JSON-RPC:
  • GraphQL:
  • CLI:
  • Rust SDK:
  • Indexing Framework:

## Description

Clap `requires` clauses refer to other flags by their field name, not
how they appear as a flag.

## Test plan

The `clickhouse-sui-indexer` example failed to run because of this
issue, and now it runs.
@amnn amnn self-assigned this Dec 3, 2025
@amnn amnn requested a review from a team as a code owner December 3, 2025 11:49
@amnn amnn temporarily deployed to sui-typescript-aws-kms-test-env December 3, 2025 11:49 — with GitHub Actions Inactive
@vercel
Copy link

vercel bot commented Dec 3, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
multisig-toolkit Ignored Ignored Dec 3, 2025 11:49am
sui-kiosk Ignored Ignored Dec 3, 2025 11:49am

/// The framework ensures that tasked pipelines never commit checkpoints below the main
/// pipeline’s pruner watermark. Requires `--reader-interval-ms`.
#[arg(long, requires = "reader-interval-ms")]
#[arg(long, requires = "reader_interval_ms")]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need unit tests or at least one that tests the proper usage?

TaskArgs::parse_from(vec!["cmd", "--task", "t", "--reader-interval-ms", "0"]);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's not a bad idea -- will try it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants